Add test for checking user-facing resources can be manipulated by non-cluster-admin #3033
Conversation
kubevirt-bot
added
release-note-none
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/cc @awels |
akalenyu
changed the title
tests/rbac_test.go
Outdated
| outputAPIResources, err := f.RunKubectlCommand("api-resources", "--namespaced", "-o", "name") | ||
| Expect(err).ToNot(HaveOccurred(), "ERR: %s, OUT: %s", err, outputAPIResources) | ||
| for _, apiResource := range strings.Split(strings.TrimSpace(outputAPIResources), "\n") { | ||
| if strings.Contains(apiResource, "cdi.kubevirt.io") { |
There was a problem hiding this comment.
maybe use HasSuffix instead of Contains. limits the matches a bit.
There was a problem hiding this comment.
Makes sense, updated for now
| } | ||
| }, | ||
| Entry("[test_id:XXXX]for admin", "admin"), | ||
| Entry("[test_id:XXXX]for edit", "edit"), |
There was a problem hiding this comment.
What about the view user? They have much more limited access. But probably should still have get/list/watch on most resources.
There was a problem hiding this comment.
Good point. I took a stab at that and only "uploadtokenrequests" diverges. The cluster wide resources may need extra adjustments too
| @@ -147,6 +149,34 @@ var _ = Describe("Aggregated role in-action tests", Serial, func() { | |||
| Entry("[test_id:3949]can do everything with edit", "edit"), | |||
| ) | |||
|
|
|||
| DescribeTable("check all user facing resources can be manipulated by non-cluster-admin", func(user string) { | |||
There was a problem hiding this comment.
Maybe pass in a map or list of resources and rbac rules that a user should be allowed to do. This way we are checking all the aggregate roles and not just the admin ones.
There was a problem hiding this comment.
I think we want to avoid passing a list of resources, and by doing that, we protect the project from introducing unusable new APIs in the future
akalenyu
force-pushed
the
gate-user-facing-rbac
branch
from
4adb2dc to
7930e75
Compare
|
/hold let's see if we can gate against view user and thus cluster wide resources too |
kubevirt-bot
added
the
do-not-merge/hold
|
Issues go stale after 90d of inactivity. If this issue is safe to close now please do so with /lifecycle stale |
kubevirt-bot
added
the
lifecycle/stale
|
Stale issues rot after 30d of inactivity. If this issue is safe to close now please do so with /lifecycle rotten |
kubevirt-bot
added
lifecycle/rotten
This should gate us from introducing user-facing resources that cannot be manipulated by non-cluster-admin. Signed-off-by: Alex Kalenyuk <[email protected]>
akalenyu
force-pushed
the
gate-user-facing-rbac
branch
from
7930e75 to
9d801f3
Compare
|
Rotten issues close after 30d of inactivity. /close |
|
@kubevirt-bot: Closed this PR. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/reopen |
|
@akalenyu: Reopened this PR. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
Rotten issues close after 30d of inactivity. /close |
|
@kubevirt-bot: Closed this PR. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/reopen |
|
@akalenyu: Reopened this PR. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
Rotten issues close after 30d of inactivity. /close |
|
@kubevirt-bot: Closed this PR. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/reopen |
|
@akalenyu: Reopened this PR. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
kubevirt-bot
removed
the
lifecycle/rotten
|
@akalenyu: The following tests failed, say
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
Issues go stale after 90d of inactivity. If this issue is safe to close now please do so with /lifecycle stale |
kubevirt-bot
added
the
lifecycle/stale
What this PR does / why we need it:
This should keep us from introducing user-facing resources that cannot be
manipulated by non-cluster-admin.
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #
Special notes for your reviewer:
Release note: